-
Notifications
You must be signed in to change notification settings - Fork 9
feat(runner): Add ability to create cells without links and unify cell implementations #2009
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 10 files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed changes from recent commits (found 1 issue).
1 issue found across 7 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/runner/src/cell.ts">
<violation number="1" location="packages/runner/src/cell.ts:355">
When ensureLink() runs for a cell whose link already has a space but not an id, `_link` never gets the generated id. `hasFullLink()` stays false, so link access keeps failing. Please update the condition so `_link` is reassigned whenever the id is missing.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed changes from recent commits (found 1 issue).
1 issue found across 7 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/runner/src/cell.ts">
<violation number="1" location="packages/runner/src/cell.ts:355">
If ensureLink() runs when the link already has a space but no id, we never copy the newly created id back onto _link, so hasFullLink() keeps failing and link() throws. Make sure we also update _link when only the id is missing.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
cf0681a to
57e4f6c
Compare
…l implementations
Implements the ability to create cells before they have a full link,
allowing deferred link creation based on cause information. This also
unifies the previous RegularCell and StreamCell implementations into a
single CellImpl class.
Key architectural changes:
- Merged RegularCell and StreamCell into unified CellImpl class
- Stream detection now happens at runtime via isStream() check
- Both regular cells and streams share the same interface
- .set() automatically handles stream vs regular cell behavior
- Use NormalizedLink (with optional id/space) instead of requiring NormalizedFullLink
- Cells can start with just { path: [] } and get id/space populated later
- hasFullLink() checks for presence of both id and space fields
- ensureLink() populates missing fields from cause information
- Add .for(cause) method to associate causes with cells
- Stores cause for later link creation
- Supports optional { force } parameter for future extensions
- Returns cell for method chaining
- Deferred link creation using createRef()
- ensureLink() creates entity IDs from causes when needed
- Uses frame context or explicit .for() cause
- Helpful error messages when context is insufficient
- Update .key() to build paths incrementally
- Works with partial links (just path[])
- Inherits cause from parent cell
- Seamless transition to full links when needed
API simplification:
- Removed isStream() checks from client code (now handled internally)
- Updated createCell() signature (removed noResolve parameter)
- Consistent Cell interface regardless of stream/regular distinction
Test coverage:
- 23 new tests for optional link functionality
- All 124 existing cell tests still passing
- Full type checking passes
Rollout plan updates:
- Marked completed tasks in rollout-plan.md
- Documented implementation approach
This enables more flexible cell creation patterns where cells can be
created and manipulated before their final identity is determined,
which is essential for the recipe construction work.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Implement Strategy 1 for sharing link creation across sibling cells. When cells like .asSchema() and .withTx() create siblings, they now share a CauseContainer that holds the entity ID and cause. Key changes: - Added CauseContainer interface with id (URI) and cause fields - Each cell has own _link (space, path, schema) but shares _causeContainer - .for() sets cause in shared container, visible to all siblings - .key() creates children with new CauseContainer (different identity) - .asSchema() and .withTx() create siblings sharing CauseContainer - ensureLink() populates shared container's id from cause - Constructor signature: runtime, tx, link?, synced?, causeContainer? This allows any sibling to trigger link creation via .for() or ensureLink(), and all siblings see the created ID. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Change the .for() method to throw an error by default if a cause is
already set, with an optional boolean parameter to allow treating it
as a suggestion.
Changes:
- Renamed parameter from `options: { force?: boolean }` to `allowIfSet?: boolean`
- Default behavior: throw error if cause/link already exists
- Pass `true` to silently ignore if cause already set (treat as suggestion)
- Moved .for() signature to IAnyCell interface for consistency
- Updated tests to verify new behavior
This makes the API safer by preventing accidental overwrites while still
allowing flexibility when needed.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Audit and fix frame creation to ensure space is always available from the result cell context: 1. Modified pushFrameFromCause() to: - Extract space from unsafe_binding and set on frame.space - Accept inHandler boolean parameter (replaces event field) - This makes frame.space available as fallback in cell.ts 2. Renamed Frame.event to Frame.inHandler: - More accurate semantic: indicates handler context, not event data - Used for per-frame counter-based ID generation fallback - Simpler boolean check vs checking event truthiness 3. Updated CauseContainer to store space: - Container now holds id, space, and cause for sharing across siblings - ensureLink() checks if container has both id and space before deriving - Space from container takes precedence over frame space 4. Updated event handler in runner.ts: - Pass inHandler: true to pushFrameFromCause() - Frame now has both space (from processCell.space) and inHandler flag Benefits: - frame.space fallback in cell.ts now works correctly - clearer semantics with inHandler vs event - space always available from result cell in runner contexts - CauseContainer properly shares space across siblings
57e4f6c to
c7b5798
Compare
|
this was merged with a follow-up PR |
Implements the ability to create cells before they have a full link, allowing deferred link creation based on cause information. This also unifies the previous RegularCell and StreamCell implementations into a single CellImpl class.
Key architectural changes:
Merged RegularCell and StreamCell into unified CellImpl class
Use NormalizedLink (with optional id/space) instead of requiring NormalizedFullLink
Add .for(cause) method to associate causes with cells
Deferred link creation using createRef()
Update .key() to build paths incrementally
API simplification:
Test coverage:
Rollout plan updates:
This enables more flexible cell creation patterns where cells can be created and manipulated before their final identity is determined, which is essential for the recipe construction work.
🤖 Generated with Claude Code
Summary by cubic
Enable creating cells before they have a full link and unify RegularCell/StreamCell into a single CellImpl, simplifying the API and making streams transparent to callers.
New Features
Migration
Written for commit c7b5798. Summary will update automatically on new commits.